Skip to content

Improve formatting of datatypes#20605

Open
emilk wants to merge 10 commits intoapache:mainfrom
rerun-io:emilk/improve-errors
Open

Improve formatting of datatypes#20605
emilk wants to merge 10 commits intoapache:mainfrom
rerun-io:emilk/improve-errors

Conversation

@emilk
Copy link
Contributor

@emilk emilk commented Feb 27, 2026 •
edited
Loading

Which issue does this PR close?

Rationale for this change

Error messages should be friendly, short, and readable.

What changes are included in this PR?

  • Implement proper Display for dyn LogicalType, TypeSignature, TypeSignatureClass, and Coercion instead of falling back to Debug
  • Switch error messages across the codebase from {:?} (Debug) to {} (Display)
  • Add non-null prefix for non-nullable fields in NativeType Display, following Arrow's convention

The north star is to follow the convention set by arrow-rs

Are these changes tested?

New snapshot tests have been introduced, and existing ones updated.

Are there any user-facing changes?

Yes!

Before

LogicalType(Native(List(LogicalField { name: "item", logical_type: LogicalType(Native(Int32), Int32), nullable: true })), List(LogicalField { name: "item", logical_type: LogicalType(Native(Int32), Int32), nullable: true }))

After

List(Int32)

Before

No function matches the given name and argument types 'round(Utf8, Utf8)'.
  Candidate functions:
  round(Coercion(TypeSignatureClass::Native(LogicalType(Native(Float64), Float64)),
    implicit_coercion=ImplicitCoercion([Numeric], default_type=Float64)),
    Coercion(TypeSignatureClass::Native(LogicalType(Native(Int64), Int64)),
    implicit_coercion=ImplicitCoercion([Integer], default_type=Int64)))

Function 'abs' expects NativeType::Numeric but received NativeType::String

Failed to coerce arguments: coercion from Int32, Utf8 to the signature
  Coercible([Coercion { desired_type: Native(LogicalType(Native(Float64), Float64)), ... }]) failed

After

No function matches the given name and argument types 'round(Utf8, Utf8)'.
  Candidate functions:
  round(Float64, Int64)

Function 'abs' expects Numeric but received String

Failed to coerce arguments: coercion from Int32, Utf8 to the signature Coercible(Float64, Int64) failed

@github-actions github-actions bot added logical-expr Logical plan and expressions optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) common Related to common crate functions Changes to functions implementation physical-plan Changes to the physical-plan crate sql SQL Planner labels Feb 27, 2026
@timsaucer timsaucer self-requested a review February 27, 2026 19:36
@timsaucer timsaucer force-pushed the emilk/improve-errors branch from 9b2c119 to 30f7fc8 Compare February 28, 2026 12:54
@emilk emilk force-pushed the emilk/improve-errors branch from 30f7fc8 to 21e0efb Compare February 28, 2026 12:57
@emilk
Copy link
Contributor Author

emilk commented Feb 28, 2026

oooops - sorry, didn't realize you were working on this :) Feel free to force-push again, and I'll go clean my apartment instead, like I should be doing…

emilk and others added 8 commits February 28, 2026 08:42
@emilk @timsaucer
Add snapshot tests of existing formatting
10ef397
@emilk @claude @timsaucer
Improve Display formatting of LogicalType, TypeSignatureClass, and Co…
b264426 
…ercion

Use Arrow DataType Display style as the north-star: terse, readable type
names instead of dumping internal Rust enum structure.

- `dyn LogicalType`: delegates to NativeType Display instead of Debug,
  e.g. `Int32` instead of `LogicalType(Native(Int32), Int32)`
- `TypeSignatureClass`: shows just the class name,
  e.g. `Numeric` instead of `TypeSignatureClass::Numeric`
- `Coercion`: shows just the desired type,
  e.g. `Float64` instead of `Coercion(TypeSignatureClass::...)`
- `ImplicitCoercion`: uses Display for its fields instead of Debug

This dramatically improves error messages like:
  Before: `round(Coercion(TypeSignatureClass::Native(LogicalType(Native(Float64), Float64)), implicit_coercion=ImplicitCoercion([Numeric], default_type=Float64), ...)`
  After:  `round(Float64, Int64)`

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@emilk @timsaucer
Add non-null prefix
ef9408d
@emilk @timsaucer
Use Display formatting for errors
bbb7ed3
@emilk @timsaucer
cargo fmt
4b2f97d
@emilk @timsaucer
A few more improvements
2cd0b73
@emilk @timsaucer
Update another test
1991966
@timsaucer
Update sql tests for new error message format
aea6186
@timsaucer timsaucer force-pushed the emilk/improve-errors branch from 21e0efb to aea6186 Compare February 28, 2026 15:28
@timsaucer
Difference between ci and test profiles generates different results o…
013abab 
…n the snapshots, so just check the first part of the error message
@timsaucer
minor test update
dc6910e
timsaucer
timsaucer approved these changes Mar 1, 2026
View reviewed changes
Copy link
Member

@timsaucer timsaucer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice cleanup! I'll leave it open for one more working day in case anyone else wants to chime in.

@Jefffrey Jefffrey mentioned this pull request Mar 2, 2026
Open
@timsaucer
Copy link
Member

timsaucer commented Mar 2, 2026

@Jefffrey any opposition to merging this in? I see you also have #20070 which touches on some of the same things

@Jefffrey
Copy link
Contributor

Jefffrey commented Mar 2, 2026

I'll try take a look at this tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timsaucer timsaucer timsaucer approved these changes

Assignees

No one assigned

Labels

common Related to common crate functions Changes to functions implementation logical-expr Logical plan and expressions optimizer Optimizer rules physical-plan Changes to the physical-plan crate sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve error messages when coercion fails

3 participants

@emilk @timsaucer @Jefffrey